feat!: internally maintain provider status #276

toddbaert · 2024-06-12T18:39:06Z

This PR implements a few things from spec 0.8.0:

implements internal provider status (already implemented in JS)
- the provider no longer updates its status to READY/ERROR, etc after init (the SDK does this automatically)
- the provider's state is updated according to the last event it fired
adds PROVIDER_FATAL error and code
adds "short circuit" feature when evaluations are skipped if provider is NOT_READY or FATAL
removes some deprecations that were making the work harder since we already have pending breaking changes.

Fixes: #250

src/OpenFeature/Api.cs

src/OpenFeature/EventExecutor.cs

src/OpenFeature/OpenFeatureClient.cs

src/OpenFeature/ProviderRepository.cs

src/OpenFeature/FeatureProvider.cs

kinyoklion · 2024-06-12T19:36:59Z

Maybe more related to the spec than to this specific implementation. But I did find it somewhat hard to handle the runtime status after initialization status when the SDK automatically implements ready.

I think it makes it easy for simple providers, and somewhat cumbersome for more complex providers. I would actually prefer to be able to opt out.

Basically an underlying SDK may already emit these events, but we have to connect and translate these events conditionally around initialization. So after initialization if a provider fails we want to emit that event, but if during initialization it fails we need to not emit that event from the provider because the SDK also will emit it.

Example of attempting to reconcile this in a python provider: https://github.com/launchdarkly/openfeature-python-server/blob/243a8dc515bcd16fe8c7d5a0913fbdfe6e073aa0/ld_openfeature/provider.py#L46

toddbaert · 2024-06-12T19:56:35Z

So after initialization if a provider fails we want to emit that event, but if during initialization it fails we need to not emit that event from the provider because the SDK also will emit it.

I can see the issue here. We have some similar logic in flagd since it also supports automatic reconnection. I think for the majority of providers though, such events around reconnection etc, are not supported; it does indeed seem to be something featured in the most rigorous implementations.

I'm open to some kind of opt-out for emitting events on READY, but I'm not sure if the ROI is there. 🤔 I suppose it would be fairly easy to implement. 🤔

kinyoklion · 2024-06-12T20:08:33Z

So after initialization if a provider fails we want to emit that event, but if during initialization it fails we need to not emit that event from the provider because the SDK also will emit it.

I can see the issue here. We have some similar logic in flagd since it also supports automatic reconnection. I think for the majority of providers though, such events around reconnection etc, are not supported; it does indeed seem to be something featured in the most rigorous implementations.

I'm open to some kind of opt-out for emitting events on READY, but I'm not sure if the ROI is there. 🤔 I suppose it would be fairly easy to implement. 🤔

I think an opt-out would work.

I've not thought about it extensively, but another option may be to have automatic events be part of a base class that more complex providers could choose not to use instead of it being in the API. Though that path isn't the most sustainable unless you have a language that supports more "mixin" type classes. So maybe not.

Maybe also we can leave it as is and think if the situation can be improved. Overall I feel like getting initialization correct has become more complex, so maybe the abstraction needs adjusted some.

toddbaert · 2024-06-13T15:26:09Z

So after initialization if a provider fails we want to emit that event, but if during initialization it fails we need to not emit that event from the provider because the SDK also will emit it.

I can see the issue here. We have some similar logic in flagd since it also supports automatic reconnection. I think for the majority of providers though, such events around reconnection etc, are not supported; it does indeed seem to be something featured in the most rigorous implementations.
I'm open to some kind of opt-out for emitting events on READY, but I'm not sure if the ROI is there. 🤔 I suppose it would be fairly easy to implement. 🤔

I think an opt-out would work.

I've not thought about it extensively, but another option may be to have automatic events be part of a base class that more complex providers could choose not to use instead of it being in the API. Though that path isn't the most sustainable unless you have a language that supports more "mixin" type classes. So maybe not.

Maybe also we can leave it as is and think if the situation can be improved. Overall I feel like getting initialization correct has become more complex, so maybe the abstraction needs adjusted some.

I thought a bit about an opt-out. I think we could add some fields/metadata to the provider to signal this and possibly other behaviors. Perhaps we could open an new issue to discuss this in the spec.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

toddbaert · 2024-06-17T18:21:49Z

I had to rebase this and resolve conflicts on the recently merged concellationToken/Async chnage.

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

src/OpenFeature/OpenFeatureClient.cs

askpt

I added a few comments and concerns about the ValueTask.

src/OpenFeature/Api.cs

src/OpenFeature/Model/ProviderEvents.cs

src/OpenFeature/ProviderRepository.cs

Co-authored-by: André Silva <2493377+askpt@users.noreply.github.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

src/OpenFeature/Api.cs

askpt

LGTM. I don't have any more concerns about this PR. Thanks @toddbaert

Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

🤖 I have created a release *beep* *boop* --- ## [2.0.0](v1.5.0...v2.0.0) (2024-08-21) Today we're announcing the release of the OpenFeature SDK for .NET, v2.0! This release contains several ergonomic improvements to the SDK, which .NET developers will appreciate. It also includes some performance optimizations brought to you by the latest .NET primitives. For details and migration tips, check out: https://openfeature.dev/blog/dotnet-sdk-v2 ### ⚠ BREAKING CHANGES * domain instead of client name ([#294](#294)) * internally maintain provider status ([#276](#276)) * add CancellationTokens, ValueTasks hooks ([#268](#268)) * Use same type for flag metadata and event metadata ([#241](#241)) * Enable nullable reference types ([#253](#253)) ### 🐛 Bug Fixes * Add missing error message when an error occurred ([#256](#256)) ([949d53c](949d53c)) * Should map metadata when converting from ResolutionDetails to FlagEvaluationDetails ([#282](#282)) ([2f8bd21](2f8bd21)) ### ✨ New Features * add CancellationTokens, ValueTasks hooks ([#268](#268)) ([33154d2](33154d2)) * back targetingKey with internal map ([#287](#287)) ([ccc2f7f](ccc2f7f)) * domain instead of client name ([#294](#294)) ([4c0592e](4c0592e)) * Drop net7 TFM ([#284](#284)) ([2dbe1f4](2dbe1f4)) * internally maintain provider status ([#276](#276)) ([63faa84](63faa84)) * Use same type for flag metadata and event metadata ([#241](#241)) ([ac7d7de](ac7d7de)) ### 🧹 Chore * cleanup code ([#277](#277)) ([44cf586](44cf586)) * **deps:** Project file cleanup and remove unnecessary dependencies ([#251](#251)) ([79def47](79def47)) * **deps:** update actions/upload-artifact action to v4.3.3 ([#263](#263)) ([7718649](7718649)) * **deps:** update actions/upload-artifact action to v4.3.4 ([#278](#278)) ([15189f1](15189f1)) * **deps:** update actions/upload-artifact action to v4.3.5 ([#291](#291)) ([00e99d6](00e99d6)) * **deps:** update codecov/codecov-action action to v4 ([#227](#227)) ([11a0333](11a0333)) * **deps:** update codecov/codecov-action action to v4.3.1 ([#267](#267)) ([ff9df59](ff9df59)) * **deps:** update codecov/codecov-action action to v4.5.0 ([#272](#272)) ([281295d](281295d)) * **deps:** update dependency benchmarkdotnet to v0.14.0 ([#293](#293)) ([aec222f](aec222f)) * **deps:** update dependency coverlet.collector to v6.0.2 ([#247](#247)) ([ab34c16](ab34c16)) * **deps:** update dependency coverlet.msbuild to v6.0.2 ([#239](#239)) ([e654222](e654222)) * **deps:** update dependency dotnet-sdk to v8.0.204 ([#261](#261)) ([8f82645](8f82645)) * **deps:** update dependency dotnet-sdk to v8.0.301 ([#271](#271)) ([acd0385](acd0385)) * **deps:** update dependency dotnet-sdk to v8.0.303 ([#275](#275)) ([871dcac](871dcac)) * **deps:** update dependency dotnet-sdk to v8.0.400 ([#295](#295)) ([bb4f352](bb4f352)) * **deps:** update dependency githubactionstestlogger to v2.4.1 ([#274](#274)) ([46c2b15](46c2b15)) * **deps:** update dependency microsoft.net.test.sdk to v17.10.0 ([#273](#273)) ([581ff81](581ff81)) * **deps:** update dotnet monorepo ([#218](#218)) ([bc8301d](bc8301d)) * **deps:** update xunit-dotnet monorepo ([#262](#262)) ([43f14cc](43f14cc)) * **deps:** update xunit-dotnet monorepo ([#279](#279)) ([fb1cc66](fb1cc66)) * **deps:** update xunit-dotnet monorepo to v2.8.1 ([#266](#266)) ([a7b6d85](a7b6d85)) * Enable nullable reference types ([#253](#253)) ([5a5312c](5a5312c)) * in-memory UpdateFlags to UpdateFlagsAsync ([#298](#298)) ([390205a](390205a)) * prompt 2.0 ([9b9c3fd](9b9c3fd)) * Support for determining spec support for the repo ([#270](#270)) ([67a1a0a](67a1a0a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com>

This PR implements a few things from spec 0.8.0: - implements internal provider status (already implemented in JS) - the provider no longer updates its status to READY/ERROR, etc after init (the SDK does this automatically) - the provider's state is updated according to the last event it fired - adds `PROVIDER_FATAL` error and code - adds "short circuit" feature when evaluations are skipped if provider is `NOT_READY` or `FATAL` - removes some deprecations that were making the work harder since we already have pending breaking changes. Fixes: open-feature#250 --------- Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: André Silva <2493377+askpt@users.noreply.github.com> Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Signed-off-by: Artyom Tonoyan <artonoyan@servicetitan.com>

🤖 I have created a release *beep* *boop* --- ## [2.0.0](open-feature/dotnet-sdk@v1.5.0...v2.0.0) (2024-08-21) Today we're announcing the release of the OpenFeature SDK for .NET, v2.0! This release contains several ergonomic improvements to the SDK, which .NET developers will appreciate. It also includes some performance optimizations brought to you by the latest .NET primitives. For details and migration tips, check out: https://openfeature.dev/blog/dotnet-sdk-v2 ### ⚠ BREAKING CHANGES * domain instead of client name ([open-feature#294](open-feature#294)) * internally maintain provider status ([open-feature#276](open-feature#276)) * add CancellationTokens, ValueTasks hooks ([open-feature#268](open-feature#268)) * Use same type for flag metadata and event metadata ([open-feature#241](open-feature#241)) * Enable nullable reference types ([open-feature#253](open-feature#253)) ### 🐛 Bug Fixes * Add missing error message when an error occurred ([open-feature#256](open-feature#256)) ([949d53c](open-feature@949d53c)) * Should map metadata when converting from ResolutionDetails to FlagEvaluationDetails ([open-feature#282](open-feature#282)) ([2f8bd21](open-feature@2f8bd21)) ### ✨ New Features * add CancellationTokens, ValueTasks hooks ([open-feature#268](open-feature#268)) ([33154d2](open-feature@33154d2)) * back targetingKey with internal map ([open-feature#287](open-feature#287)) ([ccc2f7f](open-feature@ccc2f7f)) * domain instead of client name ([open-feature#294](open-feature#294)) ([4c0592e](open-feature@4c0592e)) * Drop net7 TFM ([open-feature#284](open-feature#284)) ([2dbe1f4](open-feature@2dbe1f4)) * internally maintain provider status ([open-feature#276](open-feature#276)) ([63faa84](open-feature@63faa84)) * Use same type for flag metadata and event metadata ([open-feature#241](open-feature#241)) ([ac7d7de](open-feature@ac7d7de)) ### 🧹 Chore * cleanup code ([open-feature#277](open-feature#277)) ([44cf586](open-feature@44cf586)) * **deps:** Project file cleanup and remove unnecessary dependencies ([open-feature#251](open-feature#251)) ([79def47](open-feature@79def47)) * **deps:** update actions/upload-artifact action to v4.3.3 ([open-feature#263](open-feature#263)) ([7718649](open-feature@7718649)) * **deps:** update actions/upload-artifact action to v4.3.4 ([open-feature#278](open-feature#278)) ([15189f1](open-feature@15189f1)) * **deps:** update actions/upload-artifact action to v4.3.5 ([open-feature#291](open-feature#291)) ([00e99d6](open-feature@00e99d6)) * **deps:** update codecov/codecov-action action to v4 ([open-feature#227](open-feature#227)) ([11a0333](open-feature@11a0333)) * **deps:** update codecov/codecov-action action to v4.3.1 ([open-feature#267](open-feature#267)) ([ff9df59](open-feature@ff9df59)) * **deps:** update codecov/codecov-action action to v4.5.0 ([open-feature#272](open-feature#272)) ([281295d](open-feature@281295d)) * **deps:** update dependency benchmarkdotnet to v0.14.0 ([open-feature#293](open-feature#293)) ([aec222f](open-feature@aec222f)) * **deps:** update dependency coverlet.collector to v6.0.2 ([open-feature#247](open-feature#247)) ([ab34c16](open-feature@ab34c16)) * **deps:** update dependency coverlet.msbuild to v6.0.2 ([open-feature#239](open-feature#239)) ([e654222](open-feature@e654222)) * **deps:** update dependency dotnet-sdk to v8.0.204 ([open-feature#261](open-feature#261)) ([8f82645](open-feature@8f82645)) * **deps:** update dependency dotnet-sdk to v8.0.301 ([open-feature#271](open-feature#271)) ([acd0385](open-feature@acd0385)) * **deps:** update dependency dotnet-sdk to v8.0.303 ([open-feature#275](open-feature#275)) ([871dcac](open-feature@871dcac)) * **deps:** update dependency dotnet-sdk to v8.0.400 ([open-feature#295](open-feature#295)) ([bb4f352](open-feature@bb4f352)) * **deps:** update dependency githubactionstestlogger to v2.4.1 ([open-feature#274](open-feature#274)) ([46c2b15](open-feature@46c2b15)) * **deps:** update dependency microsoft.net.test.sdk to v17.10.0 ([open-feature#273](open-feature#273)) ([581ff81](open-feature@581ff81)) * **deps:** update dotnet monorepo ([open-feature#218](open-feature#218)) ([bc8301d](open-feature@bc8301d)) * **deps:** update xunit-dotnet monorepo ([open-feature#262](open-feature#262)) ([43f14cc](open-feature@43f14cc)) * **deps:** update xunit-dotnet monorepo ([open-feature#279](open-feature#279)) ([fb1cc66](open-feature@fb1cc66)) * **deps:** update xunit-dotnet monorepo to v2.8.1 ([open-feature#266](open-feature#266)) ([a7b6d85](open-feature@a7b6d85)) * Enable nullable reference types ([open-feature#253](open-feature#253)) ([5a5312c](open-feature@5a5312c)) * in-memory UpdateFlags to UpdateFlagsAsync ([open-feature#298](open-feature#298)) ([390205a](open-feature@390205a)) * prompt 2.0 ([9b9c3fd](open-feature@9b9c3fd)) * Support for determining spec support for the repo ([open-feature#270](open-feature#270)) ([67a1a0a](open-feature@67a1a0a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com> Signed-off-by: Artyom Tonoyan <artonoyan@servicetitan.com>

toddbaert requested a review from a team as a code owner June 12, 2024 18:39

toddbaert requested review from beeme1mr, bacherfl, askpt, kinyoklion, Kavindu-Dodan, thomaspoignant and gruebel June 12, 2024 18:47